Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing updateweights example #435

Merged
merged 4 commits into from
Jan 5, 2024

Conversation

cachemoi
Copy link
Contributor

@cachemoi cachemoi commented Jan 4, 2024

Changes in this PR

The hardcoded codon table objects are no longer generated at buildtime (which makes them global variables prone to being changed by an unexpected process) but initialised as part of their constructor; NewTranslationTable. This ensures that the package examples and tests run in proper isolation (and that no consumer gets surprised when they modify a table's weights and find that they've also updated a separate, unrelated table).

This PR also makes the codonTable.Copy() function less naive, which was originally created to avoid this "global variable" problem but was too naive. Ideally we would not need to make deep copies; but it is used in CompromiseCodonTable and AddCodonTable

Why are you making these changes?

Some examples in the codon package gave different results depending if they were run on their own or if they were run as part of the whole test suite, specifically ExampleTranslationTable_UpdateWeights

// go test -run=ExampleTranslationTable_UpdateWeights ./synthesis/codon -count=1

--- FAIL: ExampleTranslationTable_UpdateWeights (0.00s)
got:
false
want:
true

// go test ./... -count=1

?       github.com/bebop/poly   [no test files]
ok      github.com/bebop/poly/align     0.149s
ok      github.com/bebop/poly/align/matrix      0.308s
ok      github.com/bebop/poly/alphabet  0.202s
ok      github.com/bebop/poly/checks    0.521s
ok      github.com/bebop/poly/clone     0.415s
ok      github.com/bebop/poly/fold      0.831s
ok      github.com/bebop/poly/io        0.735s [no tests to run]
ok      github.com/bebop/poly/io/fasta  0.939s
ok      github.com/bebop/poly/io/fastq  0.945s
ok      github.com/bebop/poly/io/genbank        0.984s
ok      github.com/bebop/poly/io/gff    0.993s
ok      github.com/bebop/poly/io/pileup 1.047s
?       github.com/bebop/poly/synthesis [no test files]
ok      github.com/bebop/poly/io/polyjson       1.001s
ok      github.com/bebop/poly/io/rebase 1.013s
ok      github.com/bebop/poly/io/slow5  1.070s
ok      github.com/bebop/poly/io/uniprot        0.926s
ok      github.com/bebop/poly/mash      0.950s
ok      github.com/bebop/poly/primers   0.958s
ok      github.com/bebop/poly/primers/pcr       1.010s
ok      github.com/bebop/poly/random    1.036s
ok      github.com/bebop/poly/seqhash   1.082s
ok      github.com/bebop/poly/synthesis/codon   0.937s
ok      github.com/bebop/poly/synthesis/fix     1.133s
ok      github.com/bebop/poly/synthesis/fragment        0.953s
ok      github.com/bebop/poly/transform 0.980s
ok      github.com/bebop/poly/transform/variants        0.942s
ok      github.com/bebop/poly/tutorials 0.967s

This was due to the fact that the generated codon tables were global variables, so the tests which ran before ExampleTranslationTable_UpdateWeights modified the (pseudorandom) sequence we got out when running table.Optimize. If you ran the test by itself it behaved as if you were running it in isolation (which was presumably intended) The expected output was therefore incorrect.

The change makes it so the tables are no longer global variables, so ExampleTranslationTable_UpdateWeights now gives the same output if it runs as part of the whole test suit or on its own

Are any changes breaking? (IMPORTANT)

No

Pre-merge checklist

All of these must be satisfied before this PR is considered
ready for merging. Mergeable PRs will be prioritized for review.

  • New packages/exported functions have docstrings.
  • New/changed functionality is thoroughly tested.
  • New/changed functionality has a function giving an example of its usage in the associated test file. See primers/primers_test.go for what this might look like.
  • Changes are documented in CHANGELOG.md in the [Unreleased] section.
  • All code is properly formatted and linted.
  • The PR template is filled out.

@TimothyStiles TimothyStiles merged commit 43c015b into bebop:main Jan 5, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants